Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to bring the Ruby codebase into compliance with StandardRB/standard-rails linting rules by applying small style refactors and removing the generated .standard_todo.yml ignore list.
Changes:
- Insert blank lines after module inclusions to satisfy
Layout/EmptyLinesAfterModuleInclusion. - Refactor a few interpolations/regexes to satisfy Standard style cops.
- Remove
.standard_todo.ymlto enforce a “no ignored offenses” lint baseline.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/requests/court_dates_spec.rb | Adds empty line after include for lint compliance. |
| spec/requests/case_court_reports_spec.rb | Adds empty line after include for lint compliance. |
| spec/models/case_court_report_spec.rb | Adds empty line after include for lint compliance. |
| spec/controllers/concerns/accessible_spec.rb | Adds empty line after include for lint compliance. |
| lib/tasks/example_recurring_task.rb | Adds empty line after include for lint compliance. |
| app/services/short_url_service.rb | Adds empty line after includes for lint compliance. |
| app/notifications/delivery_methods/sms.rb | Adds empty line after include for lint compliance. |
| app/models/concerns/api.rb | Adds empty line after concern setup for lint compliance. |
| app/models/case_contact.rb | Adds empty line after include for lint compliance. |
| app/models/casa_org.rb | Adds empty line after prepend for lint compliance. |
| app/helpers/phone_number_helper.rb | Simplifies regex character class to remove redundant escapes. |
| app/decorators/contact_type_decorator.rb | Adds empty line after include for lint compliance. |
| app/datatables/volunteer_datatable.rb | Refactors interpolation in SQL fragment to satisfy style lint. |
| app/controllers/users/sessions_controller.rb | Adds empty line after include for lint compliance. |
| app/controllers/imports_controller.rb | Adds empty line after include for lint compliance. |
| app/controllers/emancipation_checklists_controller.rb | Adds empty line after include for lint compliance. |
| app/controllers/casa_cases_controller.rb | Refactors CSV filename interpolation to satisfy style lint. |
| app/controllers/all_casa_admins/sessions_controller.rb | Adds empty line after include for lint compliance. |
| .standard_todo.yml | Removes the Standard-generated todo/ignore list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -184,7 +184,7 @@ def case_contact_csv_name(case_contacts) | |||
| casa_case_number = case_contacts&.first&.casa_case&.case_number | |||
| current_date = Time.now.strftime("%Y-%m-%d") | |||
There was a problem hiding this comment.
Now that .standard_todo.yml is removed, Rails/TimeZone will fail here (and also in the XLSX filename in #show) because Time.now bypasses Rails time zones. Use Time.current / Time.zone.now consistently for formatting dates in filenames.
| current_date = Time.now.strftime("%Y-%m-%d") | |
| current_date = Time.current.strftime("%Y-%m-%d") |
| bool_filter filter do | ||
| "transition_aged_youth_cases.volunteer_id IS #{(filter[0] == "true") ? "NOT" : nil} NULL" | ||
| "transition_aged_youth_cases.volunteer_id IS #{"NOT" if filter[0] == "true"} NULL" | ||
| end |
There was a problem hiding this comment.
This change fixes the empty-string interpolation, but with .standard_todo.yml removed the file still contains a Rails/CompactBlank offense (e.g., filter.select(&:present?) in supervisor_filter). Consider switching to filter.compact_blank (and adjusting the surrounding logic) so Standard passes without the todo file.
| def strip_unnecessary_characters(number) | ||
| number.gsub(/[()\+\s\-\.]/, "") | ||
| number.gsub(/[()+\s\-.]/, "") | ||
| end |
There was a problem hiding this comment.
This regex cleanup looks fine, but with .standard_todo.yml removed this helper still has a Rails/Blank offense earlier (number.nil? || number.empty?). Consider using number.blank? (or number.to_s.blank?) so Standard passes without reintroducing the todo file.
| class Users::SessionsController < Devise::SessionsController | ||
| include Accessible | ||
|
|
||
| skip_before_action :check_user, only: :destroy |
There was a problem hiding this comment.
With .standard_todo.yml removed, this controller will still trigger Rails/LexicallyScopedActionFilter because it uses skip_before_action with only:. Refactor to avoid lexically scoped skipping (e.g., skip_before_action :check_user and then re-add before_action :check_user, except: :destroy, or move the condition into the original check_user callback).
| skip_before_action :check_user, only: :destroy | |
| skip_before_action :check_user | |
| before_action :check_user, except: :destroy |
| class AllCasaAdmins::SessionsController < Devise::SessionsController | ||
| include Accessible | ||
|
|
||
| skip_before_action :check_user, only: :destroy |
There was a problem hiding this comment.
With .standard_todo.yml removed, this controller will still trigger Rails/LexicallyScopedActionFilter because it uses skip_before_action with only:. Refactor to avoid lexically scoped skipping (e.g., skip_before_action :check_user and then re-add before_action :check_user, except: :destroy, or move the condition into the original check_user callback).
| skip_before_action :check_user, only: :destroy | |
| skip_before_action :check_user | |
| before_action :check_user, except: :destroy |
No description provided.